-
-
Notifications
You must be signed in to change notification settings - Fork 12
Thumbs #134
base: master
Are you sure you want to change the base?
Thumbs #134
Conversation
Things to fix:
There may be more bugs, but I'm not founding them yet. I think you have a lot to fix between the two branches either way. |
So, a couple of question about the decision you made here:
There is no correct answer for these questions, I just think that you should have an answer for them and understand what are the pros and cons of each solution. |
[I will answer 1st comment later on after making some changes]
|
Two things that I need fix ASAP to continue testing:
If you are not getting error 1, maybe you should try cloning SciPy master, ./quickstart.py, the checkout this branch and migrate (you will have to fake the first comment migration probably... that's something that should be added to quickstart - it could be in the same branch you are fixing the search problem). |
Ok, got voting to work. Number 2 still holds, though. Other things to improve:
That's a lot to do for now. |
[2] The JS code waits for server to complete request. So, we see small lag which sometimes can be observed clearly. This kind of method ensures
We can also do it in the other way Please let me know if you have any better idea. |
As I said, I think it would be better to do it the other way. As it's not On 13 September 2013 18:16, Surya Kasturi [email protected] wrote:
|
[1] I have observed it but didn't know the reason. But after making other changes, I didn't see it [3] Roughly, its taking 250-400ms for a vote. Do you think its huge?. I tried to reduce number of queryset iterations required while counting number of up votes, down-votes. I think this should help a bit but not too much. [4] I removed the arrows. Its clear that way [5] It does not make sense voting their own submissions. Besides, if a person has 50 submissions, and votes on each of them, it gives him 250 reputation on his profile!! It seems to be considerable abuse [7] Right now, we only see latest revision on home page while search queries contain all revisions! [8] I have actually posted about it in mailinglist when I started thumbs but it didn't get any considerable discussion. I think if we are going to create revisions even for a typo in existing submission, carrying reputation makes sense. Otherwise, it may not be necessary. [12][13] They are ordered based on wilson score. If you have changed "reputation" field from admin, try changing "score" fields and see. It has to work. Technically, the score fields range from 0-1 [15] This data is created based on "pagehits" app and its data. Sometimes, if we make less hits on our db, we don't see any submissions there. However, if there are hits and we delete respective revision from db, the homepage raises error. This has to be fixed though. |
scipy_central/submission/models.py
Outdated
- (1 * scale.REVISION_VOTE['thumb']['down']) | ||
return score | ||
|
||
def update_reputation(self, vote, prev_vote, created): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the created
argument really necessary? As I see it, when prev_vote == None
then created == False
. Instead of if created:
you can have if prev_vote is None:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its necessary. created
is not always False when prev_vote is None.
created
is True only when a new object is created. i.e., the very first instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a new object is created, as there is no previous vote, prev_vote
will be None. If it's not the very first instance, i.e., an object was not created, prev_vote
will exist and it will not be None (or if it's None, for the calculation it's all the same).
1. Update `Thumbs` model to generic 2. add `reputation` field to Revision model 3. overwrite admin actions in Thumbs model 4. `calculate_reputation()` model method to update `reputation` field 5. `set_reputation()` can be used to reset reputation field. Can only be used from command line and not used anywhere in code 6. `.spc-popover` class for bootstrap popoverENH: reputation for revision obj
There is only "upvote" for comments. Downvote is not allowed reputation onclick event is directly mentioned in list.html
1. New `score` field added to Revision model, which stores wilson, score confidence 2. add new fields to haystack search_indexes 3. Filter "tag" search based on score and reputation
1. create another tab 'top rated' on front page to show submissions based on score! Filter is created for it Note: only revision having top Wilson score is showed 2. show all submissions based on score The above "note" is applicable to [2] 3. create model property in Submission model to get top_revision of the respective submission
1. `reputation` field in user profile is present before but never used! 2. reputation for user profile is done as soon as successful voting is done
icon-arrow-up --> icon-minus icon-arrow-down --> icon-plus icon float right Used in ".accordion-group" to toggle sections
Create `enable_reputation` field in attached objects to thumbs Voting is possible if field set to True
Show user's votes on their profile page. This tab is visible only for themselves and not others
|
@@ -32,6 +35,9 @@ body { | |||
.pointer { | |||
cursor: pointer; | |||
} | |||
.block-inline { | |||
display: inline-block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this repeated again? The same for pointer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "less" compilation is making this mistake. There seems to be quite a bit duplicate code.
We can create a new PR removing this error and correcting it.
Actually, we can't remove these css files from repo and only depend on less files each time to compile..
The reason is, these css files come under static files. {{static}}
template tag won't work if we remove.
Returns: | ||
updated reputation for Revision object | ||
This can be used as | ||
obj.reputation = obj.update_reputation(vote, prev_vote, created) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the method changed.
I'm not sure if I already said it, but have you tried creating indexes for Thumbs to see if we can improve the performance? |
|
||
var $SPC_POPOVER = $('.spc-popover'); | ||
|
||
function show_vote(thumb_as, thumb_for, target, other, vote_count) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If target
and other
are jQuery objects, their name should start with $
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay
Ok, I think that's it for now. |
@@ -994,7 +995,7 @@ def show_items(request, what_view='', extra_info=''): | |||
extra_info = '' | |||
entry_order = list(all_revs) | |||
elif what_view == 'show' and extra_info == 'all-unique-revisions': | |||
all_subs = models.Submission.objects.all().order_by('-date_created') | |||
all_subs = models.Submission.objects.all().order_by('-score', '-reputation', '-date_created') | |||
page_title = 'All submissions' | |||
extra_info = ' (only showing the latest revision)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. In that case, I will take a look as to what we are getting here.
But entry_order = [sub.last_revision for sub in all_subs if sub.last_revision.is_displayed]
clearly says that only latest revision is showed..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said earlier, I thought this was another page. This was in particular is giving an error.
👍 |
Maybe something can be borrowed from: https://faq.i3wm.org/questions/ |
Reputation system for the site